[PECO-1116] Change version of GO being used in Github Action#169
Merged
Conversation
Signed-off-by: nithinkdb <nithin.krishnamurthi@databricks.com>
nithinkdb
requested review from
andrefurlan-db,
jadewang-db,
rcypher-databricks,
susodapop and
yunbodeng-db
as code owners
September 27, 2023 22:59
rcypher-databricks
approved these changes
Sep 27, 2023
mani-mathur-arch
added a commit
that referenced
this pull request
Jul 18, 2026
The consolidated branch now carries the log-callback forwarding (log_callback.go references kernel_set_log_callback / kernel_log_callback), so KERNEL_REV must point at a kernel revision that exports those symbols. Bump from the tier2 rev 02c0a434 (pre-log-callback) to 946c263, the head of kernel PR #169 (mani/c-abi-log-callback), which defines them. The kernel-lib build resolves this unmerged SHA via its PR-head-ref fallback, so the tagged kernel-backend build links a header carrying the symbols. Temporary: re-pin to the squash-merge SHA once kernel #169 lands. Co-authored-by: Isaac Signed-off-by: Mani Kaustubh Mathur <mani.mathur@databricks.com>
mani-mathur-arch
added a commit
that referenced
this pull request
Jul 19, 2026
Point KERNEL_REV at c19e5d1, the current head of kernel PR #169. The prior pin (946c263) was an orphaned SHA from an earlier force-push of the same PR branch. ABI version is unchanged (still 1 — the earlier bump was reverted), and all driver-used C-ABI symbols are present in the header at the new rev. Co-authored-by: Isaac
mani-mathur-arch
added a commit
that referenced
this pull request
Jul 19, 2026
Point KERNEL_REV at c19e5d1, the current head of kernel PR #169. The prior pin (946c263) was an orphaned SHA from an earlier force-push of the same PR branch. ABI version is unchanged (still 1 — the earlier bump was reverted), and all driver-used C-ABI symbols are present in the header at the new rev. Co-authored-by: Isaac Signed-off-by: Mani Kaustubh Mathur <mani.mathur@databricks.com>
mani-mathur-arch
added a commit
that referenced
this pull request
Jul 20, 2026
Match the slimmed kernel PR (#169 → mTLS + ABI-version only): remove the Go wiring for the C-ABI symbols dropped kernel-side, re-pin KERNEL_REV to the updated kernel head (12919b3), and keep the two survivors. Keep: - WithKernelClientCertificate → set_tls_client_certificate (mTLS); the option, config plumbing, unit tests, and kernel_mtls_test.go hermetic handshake test all stay. - checkABIVersion() / abiVersions() at connect (abi.go + abi_test.go + TestABIVersionMatches) — the drift handshake. Remove (no merged consumer; kernel symbols gone): - ctx-cancel bridge: cancel.go (ctxWatcher/cancelledErr), cancel_test.go, cancel_connect_test.go; revert OpenSession to plain kernel_session_open, rows.go nextBatch to plain kernel_result_stream_next_batch, and operation.go's execute-path wrap to its inline form. - log-callback bridge: log_callback.go, logforward.go, logforward_test.go, and the TestLogCallback* cases; initKernelLogging reverts to the kernel_init_logging (stderr) path. - CloudFetch toggle: WithKernelCloudFetch + the CloudFetchEnabled field and its validation/forwarding/classification (UseCloudFetch back to inert). - close_blocking references (never wired; CloseSession stays fire-and-forget). Reverted kernel_e2e_test.go to drop the three cancel-family e2e tests (mid-fetch / query-cancel-during-execution / connect-honors-cancelled-ctx); the pre-existing TestKernelE2ECancellation still covers query cancel. Validated against the new kernel lib: untagged build/vet/tests green; tagged build + vet + kernel-backend unit tests green; live e2e against a warehouse green (Select1, DataTypes ×18, CloudFetch, Cancellation, M2M, TLSSkipVerify, Interval, MetricViewMetadata — all PASS). Co-authored-by: Isaac Signed-off-by: Mani Kaustubh Mathur <mani.mathur@databricks.com>
mani-mathur-arch
added a commit
that referenced
this pull request
Jul 21, 2026
Bring PR #409 up to date after #412 (SEA PuPr tail: proxy / retry / max-chunks / telemetry) and #414 (ErrorCategory) landed on main. #409 and #412 both branched off #399 and extend the same kernel option surface, so the merge conflicted on 8 files; every conflict is additive-vs-additive and resolved by keeping both sides: - KERNEL_REV — re-pin to the #169 kernel tip 760625b, the only rev that carries BOTH #169's symbols (mTLS set_tls_client_certificate + abi_version, which #409's own code links) AND #178's set_retry_config (which the merged-in #412 code links). main's 9ac3f3d lacks the former; the old pin 12919b3 lacks the latter. 760625b = 12919b3's work + merged #178, same #169 lineage. - internal/config/config.go, internal/backend/kernel/config.go, connector.go, doc.go, kernel_config.go — keep both option families (mTLS client cert from #409; proxy / retry-overall-timeout / max-chunks from #412). The connector reject message and the ProxyURL doc take main's forward-looking wording. - kernel_test.go, kernel_experimental_test.go — de-interleave the spliced test additions (ABI-version test vs proxy/retry tests; disposition-map, option, and DeepCopy cases) into complete units, keeping every test from both. Verified on BOTH build paths: default pure-Go (go build/test ./...) and tagged (-tags databricks_kernel, linking the .a rebuilt at 760625b) — build, vet, and the touched-package tests all green. TestKernelExperimentalFieldsClassified passes, confirming no experimental field was dropped in the merge. Co-authored-by: Isaac Signed-off-by: Mani Kaustubh Mathur <mani.mathur@databricks.com>
mani-mathur-arch
added a commit
that referenced
this pull request
Jul 21, 2026
The kernel-backend CI job started failing after this branch re-pinned KERNEL_REV onto the #169 lineage that carries #178's configurable retry policy (5 attempts, 1s..60s backoff, 900s budget), which is now honored on the kernel path via WithRetries. The hermetic TLS tests (kernel_mtls_test.go) assert the TLS/mTLS handshake OUTCOME against a local httptest server. Their negative cases (handshake must fail: no client cert, untrusted CA, hostname mismatch) drive a connect that is meant to fail — and now retry it with escalating backoff. kernel_session_open is a blocking cgo call that cannot observe the Go ctx cancel mid-retry (a tracked deferral), so runKernelConnect's query goroutine outlived the test by ~34s per negative case and eventually wrote to the torn-down test pipe → "signal: broken pipe", failing the root package. Add WithRetries(-1, 0, 0) — the idiomatic disable form, honored on the kernel path — to runKernelConnect so a deliberately-failing connect settles after a single attempt. The whole file now runs in ~0.15s (was tens of seconds per negative case). No production code change; only the test's connect options. Co-authored-by: Isaac Signed-off-by: Mani Kaustubh Mathur <mani.mathur@databricks.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
We need to change the version that we have in our Github action and bump the requirements in the go registry. (May also need to do major version change)